Skip to content

testproxy: forward raw error body and headers from upstream#5263

Merged
pietern merged 1 commit into
mainfrom
testserver-proxy-fix
May 19, 2026
Merged

testproxy: forward raw error body and headers from upstream#5263
pietern merged 1 commit into
mainfrom
testserver-proxy-fix

Conversation

@pietern

@pietern pietern commented May 18, 2026

Copy link
Copy Markdown
Contributor

The reverse proxy in libs/testproxy re-marshalled apierr.APIError into a {error_code, message} envelope, dropping details[] and any other fields the workspace returned. As a result, acceptance tests run against the cloud could not observe error metadata that real CLI/TF invocations rely on.

This change forwards apiErr.ResponseWrapper.DebugBytes verbatim with the original status code, so callers see exactly what the workspace sent. As a knock-on fix, response headers in includeResponseHeaders (e.g. X-Databricks-Org-Id) are now also passed through on the error path — previously the WithResponseHeader visitors weren't invoked when apiClient.Do returned an error.

ResponseWrapper has been populated on every APIError since databricks-sdk-go#1261 (v0.100.0); the CLI is on v0.132.0. A panic guards the invariant in case the SDK ever changes shape.

The reverse proxy in libs/testproxy re-marshalled apierr.APIError into a
{error_code, message} envelope, dropping details[] and any other fields
the workspace returned. As a result, acceptance tests run against the
cloud could not observe error metadata that real CLI/TF invocations rely
on.

Forward apiErr.ResponseWrapper.DebugBytes verbatim with the original
status code so callers see exactly what the workspace sent. Also pass
through response headers in includeResponseHeaders on the error path;
WithResponseHeader visitors are not invoked when apiClient.Do returns
an error.

ResponseWrapper has been populated on every APIError since
databricks/databricks-sdk-go#1261 (v0.100.0); the CLI is on v0.132.0.
A panic guards the invariant in case the SDK ever changes shape.

Co-authored-by: Isaac
@pietern pietern added this pull request to the merge queue May 19, 2026
Merged via the queue into main with commit 9551b6b May 19, 2026
25 checks passed
@pietern pietern deleted the testserver-proxy-fix branch May 19, 2026 06:23
denik pushed a commit that referenced this pull request May 20, 2026
The reverse proxy in `libs/testproxy` re-marshalled `apierr.APIError`
into a `{error_code, message}` envelope, dropping `details[]` and any
other fields the workspace returned. As a result, acceptance tests run
against the cloud could not observe error metadata that real CLI/TF
invocations rely on.

This change forwards `apiErr.ResponseWrapper.DebugBytes` verbatim with
the original status code, so callers see exactly what the workspace
sent. As a knock-on fix, response headers in `includeResponseHeaders`
(e.g. `X-Databricks-Org-Id`) are now also passed through on the error
path — previously the `WithResponseHeader` visitors weren't invoked when
`apiClient.Do` returned an error.

`ResponseWrapper` has been populated on every `APIError` since
[databricks-sdk-go#1261](databricks/databricks-sdk-go#1261)
(v0.100.0); the CLI is on v0.132.0. A panic guards the invariant in case
the SDK ever changes shape.
bernardo-rodriguez pushed a commit to bernardo-rodriguez/b-cli that referenced this pull request May 21, 2026
…ricks#5264)

## Summary

Adds support for `replace_existing` on Lakebase `postgres_branches` and
`postgres_endpoints` so users can bring the implicitly-created
production branch and primary read-write endpoint under bundle
management — previously these always returned 409 ALREADY_EXISTS on
deploy.

Also closes the destroy-side gap: the backend signals
lifecycle-owned-by-parent via
`details[].ErrorInfo.metadata.declarative_context: MANAGED_BY_PARENT`.
The Terraform provider already honors this; this PR adds the equivalent
suppression in the direct engine. The acceptance testserver is taught
the same payload shape.

Two focused acceptance tests cover the override-implicit-resource flow
on both engines:

- `postgres_branches/replace_existing`: takes over the production
branch, applies a non-default `no_expiry: true`.
- `postgres_endpoints/replace_existing`: takes over the primary endpoint
of a user-created branch, overrides the project-inherited
`suspend_timeout_duration` (300s → 600s).

## Depends on

Built on top of databricks#5263 (testproxy: forward raw error body and headers
from upstream) — without that fix, the acceptance test proxy strips
`details[]` from upstream errors before they reach the engines, so the
suppression in TF and the direct engine never fires when tests run
against cloud.

## Test plan

- [x] Manually ran both acceptance tests against a real workspace — full
deploy + destroy on both engines.

This pull request and its description were written by Isaac.
TanishqDatabricks pushed a commit to TanishqDatabricks/cli that referenced this pull request May 22, 2026
…ks#5263)

The reverse proxy in `libs/testproxy` re-marshalled `apierr.APIError`
into a `{error_code, message}` envelope, dropping `details[]` and any
other fields the workspace returned. As a result, acceptance tests run
against the cloud could not observe error metadata that real CLI/TF
invocations rely on.

This change forwards `apiErr.ResponseWrapper.DebugBytes` verbatim with
the original status code, so callers see exactly what the workspace
sent. As a knock-on fix, response headers in `includeResponseHeaders`
(e.g. `X-Databricks-Org-Id`) are now also passed through on the error
path — previously the `WithResponseHeader` visitors weren't invoked when
`apiClient.Do` returned an error.

`ResponseWrapper` has been populated on every `APIError` since
[databricks-sdk-go#1261](databricks/databricks-sdk-go#1261)
(v0.100.0); the CLI is on v0.132.0. A panic guards the invariant in case
the SDK ever changes shape.
TanishqDatabricks pushed a commit to TanishqDatabricks/cli that referenced this pull request May 22, 2026
…ricks#5264)

## Summary

Adds support for `replace_existing` on Lakebase `postgres_branches` and
`postgres_endpoints` so users can bring the implicitly-created
production branch and primary read-write endpoint under bundle
management — previously these always returned 409 ALREADY_EXISTS on
deploy.

Also closes the destroy-side gap: the backend signals
lifecycle-owned-by-parent via
`details[].ErrorInfo.metadata.declarative_context: MANAGED_BY_PARENT`.
The Terraform provider already honors this; this PR adds the equivalent
suppression in the direct engine. The acceptance testserver is taught
the same payload shape.

Two focused acceptance tests cover the override-implicit-resource flow
on both engines:

- `postgres_branches/replace_existing`: takes over the production
branch, applies a non-default `no_expiry: true`.
- `postgres_endpoints/replace_existing`: takes over the primary endpoint
of a user-created branch, overrides the project-inherited
`suspend_timeout_duration` (300s → 600s).

## Depends on

Built on top of databricks#5263 (testproxy: forward raw error body and headers
from upstream) — without that fix, the acceptance test proxy strips
`details[]` from upstream errors before they reach the engines, so the
suppression in TF and the direct engine never fires when tests run
against cloud.

## Test plan

- [x] Manually ran both acceptance tests against a real workspace — full
deploy + destroy on both engines.

This pull request and its description were written by Isaac.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants